Re: [Xen-devel] [PATCH] pypxeboot bootloader

[Stephen Childs <Stephen.Childs@xxxxxxxxx>]

Tim Deegan wrote:
> First impressions: this looks really useful but has a few rough edges
> that need to be knocked off.  (I haven't had a chance to try this on a
> real machine yet.)
I absolutely agree, this was the first working version and I was aware 
there were nasties in it.
> > +for line in udhcplines:
> > +    s = line.strip()
> > +    f = s.split()
> > +
> > +    if s[0]=='{' and s[-1]=='}':
> > +        dhcpinfo=eval(s, {"__builtins__" : {}})
> > +        for k in dhcpinfo:
> > +            dhcpinfo[k]=dhcpinfo[k].strip()
> Executing the results of programs scares me.  Could you just parse it?
Yes quite. Sorry that was cut-and-paste code so I didn't think about it 
enough. Will rewrite.
> > +ipaddr=dhcpinfo['ip']
> > +ipaddrlist=ipaddr.split('.')
> > +hexip=commands.getstatusoutput("/usr/bin/gethostip -x "+ipaddr)[1]
> This requires syslinux to be installed on the Xen host, and isn't 
> hard to do by hand: "%2.2x%2.2x%2.2x%2.2x" % tuple(map (int, ipaddrlist))
> (I'm sure more heavyweight python hackers can correct my style.)
Yes of course I'll change that.

> > +tmpdir="/var/lib/xen/"
> > +
> > +os.chdir(tmpdir)
> > +commandstr="tftp -c get "+servaddr+":pxelinux.cfg/"+hexip
> > +#print "running command "+commandstr
> > +getpxeres=commands.getstatusoutput(commandstr)
> You'll need to be more careful with temporary file names -- here at
> least the filename is unique to the guest (we hope!) but for the kernel
> etc., two boots could clash.  Also, you don't seem to remove the
> downloaded config files.
Yes that all needs tidying up. Unfortunately the version of tftp I was 
using didn't actually support "get <remote-file> <local-file>" as 
advertised which I was initially going to use. I'll have to download to 
the filename supplied by the server then move to a tmpfile.
On the subject of removing files yes of course I'll remove the config 
files. Comparing with pygrub, that seems to leave kernels and initrds 
clogging up /var/lib/xen, and I suppose I'll end up doing the same. I 
suppose you could set up tmpwatch to clean it up, but it still seems a bit 
messy.
Expect a new patch soon ...

Stephen
--
Dr. Stephen Childs,
Research Fellow, EGEE Project,    phone:                    +353-1-8961797
Computer Architecture Group,      email:        Stephen.Childs @ cs.tcd.ie
Trinity College Dublin, Ireland   web: http://www.cs.tcd.ie/Stephen.Childs

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel