[PATCH 1/2] dracut: fix crash with PXE ipappend and ks=file:..
Will Woods
wwoods at redhat.com
Tue Jul 31 18:18:41 UTC 2012
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
More information about the anaconda-patches
mailing list