[prev in list] [next in list] [prev in thread] [next in thread]
List: xen-devel
Subject: Re: [Xen-devel] [PATCH v3 4/8] osstest: add a FreeBSD host install recipe
From: Roger Pau Monne <roger.pau () citrix ! com>
Date: 2017-06-30 17:07:37
Message-ID: 20170630170737.hre2ewl7t7dywctl () MacBook-Pro-de-Roger ! local
[Download RAW message or body]
On Fri, Jun 23, 2017 at 03:45:45PM +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH v3 4/8] osstest: add a FreeBSD host install recipe"):
> > +sub setup_netboot_installer () {
> > + my $image = "$path_prefix/install.img";
> > + my $pxeimg = target_tftp_prefix($ho) . "--freebsd.img";
> > + my $hash = `sha256sum $image | head -c 16` or die $!;
>
> You should do this ` -' stripping in Perl. That way, also, you won't
> lose the exit status from sha256sum as you do here.
I've added a pre-patch that introduces a sha256file helper in order to
calculate the hash of a file.
> > + my $tftp_freebsd = "$ho->{Tftp}{Path}/$ho->{Tftp}{TmpDir}/freebsd-images/";
> > + my $script = <<'END';
> > +basedir=$0
> > +imagepath=$1
> > +sharedpath=$2
> > +targetpath=$3
>
> Please pass a dummy argument to the script (I usually use `x')
> and shift all these arguments along. Passing a real argument as $0 is
> IMO strange.
I agree, I was surprised to find out I could do it like that.
> > +cd $basedir
> > +if [ ! -f $sharedpath ]; then
> > + mkdir -p `dirname $sharedpath`
> > + cp $imagepath $sharedpath
>
> Please use the copy-to-tempfile-and-rename pattern.
>
> AFAICT your filename pattern for the per-hash filename is
> $tftp_freebsd/$r{arch}/$hash/install.img
>
> Is there some reason why this needs
> - to be qualified with $r{arch}
> - to have a bunch of per-hash directories containing one file each
> ?
>
> Why not
> $tftp_freebsd/by-hash/$hash.img
> ?
Yes, I've changed the path now to the suggested one.
> > +# Dir format from basedir is $arch/$hash/install.img
> > +for hashdir in `find -mindepth 2 -maxdepth 2 -type d`; do
>
> 1. use xargs or -exec -rm
> 2. use find -links
> 3. add -ctime +7 or something, so we don't delete things which
> have just been added (or used)
Done:
find `dirname $sharedpath` -links 1 -ctime +7 -delete
> > +for nic in \$nics; do
> > + addr=`ifconfig \$nic inet|grep inet|awk {'print \$2'}`
> > + if [ "\$addr" = "$ho->{Ip}" ]; then
> > + echo \$nic
> > + exit 0
> > + fi
> > +done
> > +exit 1
> > +END
>
> Is it likely that the disk or network device name, for a particular
> device, will change, for example across versions of FreeBSD ?
I don't think it happens usually, but I guess the driver name could
change, and in turn make the device name change. As I said I think
this is mostly stable, so that users don't get nasty surprises.
> > + logm("Uploading the install sets to the system");
> > + target_cmd_root($ho, <<END, 30);
> > +mkdir -p $target_sets
>
> Missing set -e.
>
> > + logm("Creating the installer script");
> > + target_putfilecontents_root_stash($ho, 10, <<END, '~/installscript');
> > +set -a
> > +BSDINSTALL_DISTDIR="$target_sets"
> > +ZFSBOOT_DISKS="$disk"
> > +DISTRIBUTIONS="@sets"
> > +nonInteractive=1
> > +
> > +#!/bin/sh
> > +set -ex
>
> There's a #! halfway through this script.
Yes, that's intended, it's the format of the install script. This
first part contains variables (ie: input data) used by the installer.
The second part is automatically executed chrooted into the root dir
of the newly installed system once done.
https://www.freebsd.org/cgi/man.cgi?query=bsdinstall
See the SCRIPTING section.
> > +# Setup serial console
> > +printf "%s" "-h -S$c{Baud}" >> /boot.config
>
> Are you deliberately leaving /boot.config with no final newline ?
That's what I've usually done, but it doesn't matter.
> > +cat << ENDBOOT >> /boot/loader.conf
> > +boot_serial="YES"
> > +comconsole_speed="$c{Baud}"
> > +console="comconsole"
> > +boot_verbose="YES"
> > +beastie_disable="YES"
>
> :-) re beastie.
It just consumes serial log space and has a nasty countdown.
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic