Re: [Xen-devel] [OSSTEST Nested PATCH v11 4/7] Changes on test step of Debian hvm guest install

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

longtao.pang writes ("[OSSTEST Nested PATCH v11 4/7] Changes on test step of 
Debian hvm guest install"):
...
> 1. The default disk size for guest is '10000M' which is not sufficient
> for nested HVM guest, using larger disk size for nested guest
> to accommodate to nested test requirement, the specific disk_size is
> defined by make-flight.
> 2. In L1 installation context, assign more memory (defined in runvar) to
> it; Since it acts as a nested hypervisor anyway.
> 3. Comment out CDROM entry in sources.list to make HTTP URL entry
> available for L1 hvm guest.
> 4. Enable nestedhvm feature in 'ExtraConfig' for nested job.

I think this should be 3 separate patches:

  - Allow runvars to specify guest disk and ram size (turning
    previous values into defaults)

  - Comment out CDROM entry.  This needs better motivation.  I think
    your motivation is that the CDROM is removed and that therefore
    this line does not work ?

  - Honour $xopts{ExtraConfig} and use it to enable nestedhvm.


> +    my $extra_config='';
> +    $extra_config .="nestedhvm=1\n"
> +        if guest_var($gho,"enable_nestedhvm",'false') eq 'true';

Idiom elsewhere is   =~ m/true/   rather than  eq 'true'
although maybe Wei will provide a guest_var_boolean (see my comments
on his "[PATCH OSSTEST v3] Stubdom test case".

> +    # Use guest_var to get specific disk size, or will use default $disk_mb
> +    $disk_mb= guest_var($gho,'disksize',$disk_mb);

I would prefer this comment and the one about RAM to be expressed,
instead, like this:

    sub more_prepareguest_hvm ($$$$;@) {
        my ($ho, $gho, $ram_mb, $disk_mb, %xopts) = @_;
  +     # $ram_mb and $disk_mb are defaults, used if runvars don't say


Thanks,
Ian.

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