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

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

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; }

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.

> +    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.

Thanks,
Ian.

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