On Tue, 2012-07-31 at 10:08 +0200, Vratislav Podzimek wrote:
I don't like this patch for two reasons:
1) As I understand the (new) behaviour, using 'ipappend 2' will cause
that dracut will start the BOOTIF interface (for early networking)
instead of the device specified in the kickstart, right? I think this is
not the right behaviour, because for example our (Anaconda) PXE targets
here in Brno all contain 'ipappend 2' and it only make sense because it
is just an additional information for the installer. This does not mean
that we want to use BOOTIF interface for networking. The old behaviour
in loader was that this interface was used only when '--device=bootif'
was used in kickstart or as a default when no device was specified.
If my understanding is wrong, could you please explain the change? As I
see it, dracut should be patched to allow both "BOOTIF=..." and
"ip=..."
options.
No, your understanding is correct. This is another place where current
upstream behavior collides with Anaconda Tradition:
Anaconda Tradition:
If 'ksdevice=bootif' is used, use that device for kickstart.
Otherwise, ignore BOOTIF=.
Dracut Behavior:
Use the device specified by BOOTIF to set up the network via dhcp.
Ignore any other ip= lines, and crash if they're found.
Probably the right thing to do is to *default* to doing dhcp if BOOTIF
is found, but let kickstart 'network' commands and other 'ip' options
override it.
Dracut says:
# Don't mix BOOTIF=macaddr from pxelinux and ip= lines
getarg ip= >/dev/null && getarg BOOTIF= >/dev/null && \
die "Mixing BOOTIF and ip= lines is dangerous"
But I have no idea why this would be "dangerous". If I can't find a good
justification then we should patch dracut so we can tell it to ignore
BOOTIF if we've got kickstart config we want to use.
2) See comments below.
On Mon, 2012-07-30 at 18:30 -0400, Will Woods wrote:
> If you're using pxelinux with 'ipappend 2', it will add a
"BOOTIF=..."
> argument to your boot args.
>
> parse-kickstart will rewrite your first 'network' line to a
"ip=..."
> argument for dracut.
>
> But dracut refuses to allow "ip=..." and "BOOTIF=..." arguments
to mix,
> so this makes it die with the message:
>
> FATAL: Mixing BOOTIF and ip= lines is dangerous
>
> So: parse-kickstart should check the boot arguments, and not bother
> generating an "ip=..." line if "BOOTIF" is already there.
> ---
> dracut/parse-kickstart | 26 ++++++++++++++++++--------
> 1 file changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/dracut/parse-kickstart b/dracut/parse-kickstart
> index 3b6b861..d81dc72 100755
> --- a/dracut/parse-kickstart
> +++ b/dracut/parse-kickstart
> @@ -23,6 +23,23 @@ from collections import OrderedDict
> # Default logging: none
> log =
logging.getLogger('parse-kickstart').addHandler(logging.NullHandler())
>
> +# So we can examine commandline args
> +def readfile(f):
> + try:
> + val = open(f).readline().strip()
> + except IOError:
> + val = None
> + return val
Returning None is not a good way of an error indication, because it
results in "None type has no attribute..." tracebacks. You have to check
the returned value every time you use such function. Raising a
descriptive exception is much better way.
..oddly, the same file also has:
try:
thismac = readfile("/sys/class/net/%s/address" % netif)
except IOError:
pass
So obviously I got mixed up somewhere.
The point of readfile() was that it should return the contents of the
file, or nothing if the file is missing/empty.
Sort of like "$(cat $file 2>/dev/null)" - we don't care if the file is
missing or not.
So I'm changing it to reflect that.
New patches will be along in a bit.
-w