Re: [Xen-devel] [PATCH V4 10/12] Introduce ts-debian-hvm-install

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Apr 02, 2014 at 05:15:49PM +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH V4 10/12] Introduce ts-debian-hvm-install"):
> > This is debian hvm guest test case. It resembles ts-redhat-install:
> > 1. prepare a auto install CD
> > 2. install debian hvm guest, currently using OVMF
> > 3. test if the guest is up
> 
> Thanks.  On this patch I have only trivial comments:
> 
> > +our $stage=0;
> > +if (@ARGV && $ARGV[0] =~ m/^--stage(\d+)$/) { $stage=$1; shift @ARGV; }
> 

This comes from ts-redhat-install. My understanding is that it's used
for debugging. By specifying --stage the test can bypass certain step
such as guest installation.

> What's this business for ?  Is it just for debugging ?  In which case
> fine.
> 
> > +sub prepare_initrd ($$$) {
> > +    my ($initrddir,$newiso,$preseed_file_path) = @_;
> > +    return <<"END";
> > +      rm -rf $initrddir
> > +      mkdir $initrddir
> > +      cd $initrddir
> > +      gzip -d \< $newiso/install.amd/initrd.gz | cpio --extract 
> > --make-directories --no-absolute-filename
> 
> I'm not sure why you have the \ in front of < and > here.
> 

That's me being not familiar with perl string and too careful. Fixed.

> > +    my $newiso= "/root/$flight.$job.$gn-newiso";
> > +    my $emptydir= "/root/$flight.$job.$gn-empty-dir";
> > +    my $initrddir= "/root/$flight.$job.$gn-initrd-dir";
> > +    my $preseed_file_path = "/root/$flight.$job.$gn-preseed";
> 
> This would be less messy and repetitive with an extra variable (to
> encapsulate the prefix "/root/$flight.$job.$gn"), and/or perhaps a
> mkdir.
> 

Done.

Wei.

> Thanks,
> Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel