[OSSTEST PATCH V3] ts-debian-hvm-install: use text installer frontend

[OSSTEST PATCH V3] ts-debian-hvm-install: use text installer frontend

[Wei Liu]

Factor out di_installcmdline_base in Debian.pm and use that in
ts-debian-hvm-install. This should improve readability of d-i log in
various Debian HVM testcases.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
https://www.debian.org/releases/stable/amd64/apbs02.html.en
---
 Osstest/Debian.pm     |   40 ++++++++++++++++++++++++++++------------
 ts-debian-hvm-install |    9 +++++++--
 2 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
index ab09abb..ef508ca 100644
--- a/Osstest/Debian.pm
+++ b/Osstest/Debian.pm
@@ -36,6 +36,7 @@ BEGIN {
                       preseed_base
                       preseed_create
                       preseed_hook_command preseed_hook_installscript
+                      di_installcmdline_base
                       di_installcmdline_core
                       );
     %EXPORT_TAGS = ( );
@@ -398,6 +399,30 @@ END
 our %preseed_cmds;
 # $preseed_cmds{$di_key}[]= $cmd
 
+sub di_installcmdline_base($;@) {
+    my ($tho, %xopts) = @_;
+    my @cl = qw(
+                auto=true
+                hw-detect/load_firmware=false
+                DEBCONF_DEBUG=5
+                );
+    my $difront = get_host_property($tho,'DIFrontend','text');
+    push @cl, ("DEBIAN_FRONTEND=$difront");
+
+    die "Both PreseedURL and PreseedFile are defined."
+	if defined($xopts{PreseedURL}) && defined($xopts{PreseedFile});
+
+    push @cl, ("url=$xopts{PreseedURL}") if $xopts{PreseedURL};
+
+    push @cl, ("file=$xopts{PreseedFile}") if $xopts{PreseedFile};
+
+    my $debconf_priority= $xopts{DebconfPriority};
+    push @cl, "debconf/priority=$debconf_priority"
+        if defined $debconf_priority;
+
+    return @cl
+}
+
 sub di_installcmdline_core ($$;@) {
     my ($tho, $ps_url, %xopts) = @_;
 
@@ -405,24 +430,15 @@ sub di_installcmdline_core ($$;@) {
 
     my $netcfg_interface= get_host_property($tho,'interface force','auto');
 
-    my @cl= qw(
-               auto=true preseed
-               hw-detect/load_firmware=false
-               DEBCONF_DEBUG=5
-               );
-    my $difront = get_host_property($tho,'DIFrontend','text');
+    $xopts{PreseedURL} = $ps_url;
+
+    my @cl = di_installcmdline_base($tho, %xopts);
     push @cl, (
-               "DEBIAN_FRONTEND=$difront",
                "hostname=$tho->{Name}",
-               "url=$ps_url",
                "netcfg/dhcp_timeout=150",
                "netcfg/choose_interface=$netcfg_interface"
                );
 
-    my $debconf_priority= $xopts{DebconfPriority};
-    push @cl, "debconf/priority=$debconf_priority"
-        if defined $debconf_priority;
-
     return @cl;
 }
 
diff --git a/ts-debian-hvm-install b/ts-debian-hvm-install
index fec24f7..17adeb5 100755
--- a/ts-debian-hvm-install
+++ b/ts-debian-hvm-install
@@ -86,18 +86,23 @@ END
 
 sub grub_cfg () {
 
+    my $cmdline = join ' ' , di_installcmdline_base($gho, PreseedFile => "/preseed.cfg");
+
     return <<"END";
 set default="0"
 set timeout=5
 
 menuentry 'debian guest auto Install' {
-    linux /install.amd/vmlinuz console=vga console=ttyS0,115200n8 preseed/file=/preseed.cfg
+    linux /install.amd/vmlinuz $cmdline console=ttyS0,115200n8
     initrd /install.amd/initrd.gz
 }
 END
 }
 
 sub isolinux_cfg () {
+
+    my $cmdline = join ' ' , di_installcmdline_base($gho, PreseedFile => "/preseed.cfg");
+
     return <<"END";
     default autoinstall
     prompt 0
@@ -105,7 +110,7 @@ sub isolinux_cfg () {
 
     label autoinstall
         kernel /install.amd/vmlinuz
-        append video=vesa:ywrap,mtrr vga=788 console=ttyS0,115200n8 preseed/file=/preseed.cfg initrd=/install.amd/initrd.gz
+        append $cmdline console=ttyS0,115200n8 initrd=/install.amd/initrd.gz
 END
 }
 
-- 
1.7.10.4

Re: [OSSTEST PATCH V3] ts-debian-hvm-install: use text installer frontend

[Ian Jackson]

Wei Liu writes ("[OSSTEST PATCH V3] ts-debian-hvm-install: use text installer frontend"):
> Factor out di_installcmdline_base in Debian.pm and use that in
> ts-debian-hvm-install. This should improve readability of d-i log in
> various Debian HVM testcases.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
...
> +sub di_installcmdline_base($;@) {
> +    my ($tho, %xopts) = @_;
> +    my @cl = qw(
> +                auto=true
> +                hw-detect/load_firmware=false
> +                DEBCONF_DEBUG=5
> +                );
> +    my $difront = get_host_property($tho,'DIFrontend','text');
> +    push @cl, ("DEBIAN_FRONTEND=$difront");
> +
> +    die "Both PreseedURL and PreseedFile are defined."
> +	if defined($xopts{PreseedURL}) && defined($xopts{PreseedFile});
> +
> +    push @cl, ("url=$xopts{PreseedURL}") if $xopts{PreseedURL};
> +
> +    push @cl, ("file=$xopts{PreseedFile}") if $xopts{PreseedFile};

Frankly, I disagree with Ian and I preferred the previous arrangement
where the caller would simply specify url= or file= as the case might
be, and di_installcmdline_base would specify "auto".

This way seems like a lot of pointless work - and it's less clear,
too, because to figure out what it's doing you have to chase some more
xopts settings.

But having said that I don't object to this version either.

> diff --git a/ts-debian-hvm-install b/ts-debian-hvm-install
> index fec24f7..17adeb5 100755
> --- a/ts-debian-hvm-install
> +++ b/ts-debian-hvm-install
> @@ -86,18 +86,23 @@ END
>  
>  sub grub_cfg () {
>  
> +    my $cmdline = join ' ' , di_installcmdline_base($gho, PreseedFile => "/preseed.cfg");
> +

There seems little point having di_installcmdline_base return an
array.  The array is just for convenience of construction.  If the
caller wants such an array too then putting the whole of a
space-joined list from di_installcmdline_base would be fine, since
there is no way to quote arguments containing spaces.

That would also mean that I wouldn't have to grumble about this line
being too long :-).

Thanks,
Ian.