[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