Re: [OSSTEST Nested PATCH v9 6/9] Changes on test step of Debian hvm guest install

[Ian Campbell <ian.campbell@citrix.com>]

On Sat, 2015-05-02 at 14:28 +0800, longtao.pang wrote:
> From: "longtao.pang" <longtaox.pang@intel.com>
> 
> 1. Increase disk size to accommodate to nested test requirement.
                       ^ in preseed recipe (I think?)

> 2. Since 'Debain-xxx-.iso' image will be stored in rootfs of L1 guest,
> therefore needs more disk capacity, increase root partition size to
> 10000.
> 3. In L1 installation context, assign more memory (defined in runvar) to
> it; Since it acts as a nested hypervisor anyway.
> 4. Comment out CDROM entry in sources.list to make HTTP URL entry
> available for L1 hvm guest.
> 5. Enable nestedhvm feature in ExtraConfig for nested job.
> 
> Signed-off-by: longtao.pang <longtaox.pang@intel.com>
> ---
> Changes in v9:
> 1. Using 'guest_var' to get nested L1 guest's specific disk size and
> specific memory size.
> 2. Using 'guest_var' to enable or disable nestedhvm feature.
> ---
>  ts-debian-hvm-install |   17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/ts-debian-hvm-install b/ts-debian-hvm-install
> index fd28abc..d823233 100755
> --- a/ts-debian-hvm-install
> +++ b/ts-debian-hvm-install
> @@ -73,6 +73,7 @@ d-i apt-setup/cdrom/set-first boolean false
>  d-i preseed/late_command string \\
>          in-target mkdir -p /boot/efi/EFI/boot; \\
>          in-target cp /boot/efi/EFI/debian/grubx64.efi /boot/efi/EFI/boot/bootx64.efi ;\\
> +        in-target sed -i 's/^deb *cdrom/#&/g' /etc/apt/sources.list;

When this is rebased onto v6 of my distro testing series this will need
to become/use preseed_hook_command and you won't want to mix it with the
EFI stuff since that has/will become conditional.

That change is probably enough to invalidate any Ack I might give now,
or I would do so.

>  END
>  
>      $preseed_file .= preseed_hook_cmds();
> @@ -149,6 +150,11 @@ sub prep () {
>      target_putfilecontents_root_stash($ho, 10, preseed(),
>                                        $preseed_file_path);
>  
> +    my $extra_config='';
> +    $extra_config .="nestedhvm=1\n"
> +        # Use guest_var to get boolean value to enable nestedhvm
> +        # feature or not
> +        if guest_var($gho,"enable_nestedhvm",'false') eq 'true';

IMHO a better formatting would be:

+    # Use guest_var to get boolean value to enable nestedhvm
+    # feature or not
+    $extra_config .="nestedhvm=1\n"
+        if guest_var($gho,"enable_nestedhvm",'false') eq 'true';

Although I think you could just omit the comment, it's not adding much.

> 
>      # If host has >8G free memory, create a guest with 4G memory to catch
>      # any error that triggers cross 4G boundary
>      my $host_freemem_mb = host_get_free_memory($ho);
> @@ -157,13 +163,18 @@ sub prep () {
>      if ($host_freemem_mb > $ram_lots * 2 + $ram_minslop) {
>          $ram_mb = $ram_lots;
>      } else {
> -        $ram_mb = 768;
> +        # Use guest_var to get specific memsize, or will use default '768'
> +        $ram_mb= guest_var($gho,'memsize',768);
>      }
> -    logm("Host has $host_freemem_mb MB free memory, setting guest memory size to $ram_mb MB");
> +    logm("Host has $host_freemem_mb MB free memory,
> +         setting guest memory size to $ram_mb MB");

Blank line here please.

> +    # Use guest_var to get specific disk size, or will use default $disk_mb
> +    $disk_mb= guest_var($gho,'disksize',$disk_mb);
>  
>      more_prepareguest_hvm($ho,$gho, $ram_mb, $disk_mb,
>                            OnReboot => 'preserve',
>                            Bios => $r{bios},
> +                          ExtraConfig => $extra_config,
>                            PostImageHook => sub {
>          my $cmds = iso_copy_content_from_image($gho, $newiso);
>          $cmds .= prepare_initrd($initrddir,$newiso,$preseed_file_path);