|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 1/3] Add nested testcase of installing L1 guest VM.
On Mon, Oct 20, 2014 at 09:04:11AM +0800, longtao.pang wrote:
> From: "Robert.ho" <robert.hu@xxxxxxxxx>
>
> Adding nested test case <ts-nested-L1-debian-install> which is used for
> installing L1. Inside L1 guest VM, need build xen and HVM dom0 kernel and then
> boot into xen kernel. After that, get ready to install L2 guest VM inside L1.
>
This patch mixes mechanical changes with the actual test script. Please
break it down to smaller self-contained patches.
> ---
> Osstest/Debian.pm | 25 +-
> Osstest/TestSupport.pm | 31 ++-
> sg-run-job | 6 +
> ts-nested-L1-debian-install | 534
> +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 582 insertions(+), 14 deletions(-)
> create mode 100755 ts-nested-L1-debian-install
>
> diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
> index ab09abb..a733ac5 100644
> --- a/Osstest/Debian.pm
> +++ b/Osstest/Debian.pm
> @@ -1,5 +1,6 @@
> # This is part of "osstest", an automated testing framework for Xen.
> # Copyright (C) 2009-2013 Citrix Inc.
> +# Copyright (C) 2014 Intel Inc.
> #
> # This program is free software: you can redistribute it and/or modify
> # it under the terms of the GNU Affero General Public License as published by
> @@ -286,15 +287,18 @@ sub setupboot_grub2 ($$$) {
>
> my $count= 0;
> my $entry;
> + my $submenu;
> while (<$f>) {
> next if m/^\s*\#/ || !m/\S/;
> if (m/^\s*\}\s*$/) {
> - die unless $entry;
> + die unless $entry || $submenu;
> + if(!defined $entry && defined $submenu){
> + logm("Met end of a submenu starting from
> $submenu->{StartLine}.Our want kern is $want_kernver");
> + $submenu=undef;
> + next;
> + }
> my (@missing) =
> - grep { !defined $entry->{$_} }
> - (defined $xenhopt
> - ? qw(Title Hv KernDom0 KernVer)
> - : qw(Title Hv KernOnly KernVer));
> + grep { !defined $entry->{$_} } (defined $xenhopt ? qw(Title Hv
> KernDom0 KernVer) : qw(Title Hv KernOnly KernVer));
> if (@missing) {
> logm("(skipping entry at $entry->{StartLine};".
> " no @missing)");
> @@ -317,21 +321,24 @@ sub setupboot_grub2 ($$$) {
> $entry= { Title => $1, StartLine => $., Number => $count };
> $count++;
> }
> - if (m/^\s*multiboot\s*\/(xen\-[0-9][-+.0-9a-z]*\S+)/) {
> + if(m/^submenu\s+[\'\"](.*)[\'\"].*\{\s*$/){
> + $submenu={ StartLine =>$.};
> + }
> + if (m/^\s*multiboot\s*(?:\/boot)*\/(xen\-[0-9][-+.0-9a-z]*\S+)/) {
> die unless $entry;
> $entry->{Hv}= $1;
> }
> - if (m/^\s*multiboot\s*\/(vmlinu[xz]-(\S+))/) {
> + if (m/^\s*multiboot\s*(?:\/boot)*\/(vmlinu[xz]-(\S+))/) {
> die unless $entry;
> $entry->{KernOnly}= $1;
> $entry->{KernVer}= $2;
> }
> - if (m/^\s*module\s*\/(vmlinu[xz]-(\S+))/) {
> + if (m/^\s*module\s*(?:\/boot)*\/(vmlinu[xz]-(\S+))/) {
> die unless $entry;
> $entry->{KernDom0}= $1;
> $entry->{KernVer}= $2;
> }
> - if (m/^\s*module\s*\/(initrd\S+)/) {
> + if (m/^\s*module\s*(?:\/boot)*\/(initrd\S+)/) {
> $entry->{Initrd}= $1;
> }
> }
For example, this hunk should be a separate patch. If I'm not mistaken,
this is to make this routine parse "submenu".
> diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
> index 1d77933..30fe988 100644
> --- a/Osstest/TestSupport.pm
> +++ b/Osstest/TestSupport.pm
> @@ -55,8 +55,9 @@ BEGIN {
> target_putfilecontents_stash
> target_putfilecontents_root_stash
> target_put_guest_image target_editfile
> - target_editfile_root target_file_exists
> - target_run_apt
> + target_editfile_root target_file_exists
> + target_file_exists_root
> + target_run_apt
Please configure your editor to not alter white spaces.
If these changes are made by you as cleanup, they should come in
separate patch, too.
> target_install_packages target_install_packages_norec
> target_jobdir target_extract_jobdistpath_subdir
> target_extract_jobdistpath target_guest_lv_name
> @@ -66,7 +67,7 @@ BEGIN {
>
> selecthost get_hostflags get_host_property
> power_state power_cycle power_cycle_time
> - serial_fetch_logs
> + serial_fetch_logs select_ether
> propname_massage
>
> get_stashed open_unique_stashfile compress_stashed
> @@ -108,6 +109,7 @@ BEGIN {
> iso_gen_flags_basic
> iso_copy_content_from_image
> guest_editconfig_nocd
> + guest_editconfig_cd
And the introduction of these new helper functions should be in their
own patches.
> );
> %EXPORT_TAGS = ( );
>
> @@ -480,6 +482,14 @@ sub target_file_exists ($$) {
> die "$rfile $out ?";
> }
>
> +sub target_file_exists_root ($$) {
> + my ($ho,$rfile) = @_;
> + my $out= target_cmd_output_root($ho, "if test -e $rfile; then echo y;
> fi");
> + return 1 if $out =~ m/^y$/;
> + return 0 if $out !~ m/\S/;
> + die "$rfile $out ?";
> +}
> +
> sub teditfileex {
> my $user= shift @_;
> my $code= pop @_;
> @@ -715,6 +725,7 @@ sub power_cycle_time ($) {
> sub power_cycle ($) {
> my ($ho) = @_;
> $mjobdb->host_check_allocated($ho);
> + $mjobdb->xen_check_installed($ho);
> die "refusing to set power state for host $ho->{Name}".
> " possibly shared with other jobs\n"
> if $ho->{SharedMaybeOthers};
> @@ -911,7 +922,7 @@ sub compress_stashed($) {
> sub host_reboot ($) {
> my ($ho) = @_;
> target_reboot($ho);
> - poll_loop(40,2, 'reboot-confirm-booted', sub {
> + poll_loop(200,2, 'reboot-confirm-booted', sub {
> my $output;
> if (!eval {
> $output= target_cmd_output($ho,
> @@ -1438,7 +1449,7 @@ sub prepareguest_part_xencfg ($$$$$) {
> my $xencfg= <<END;
> name = '$gho->{Name}'
> memory = ${ram_mb}
> -vif = [ 'type=ioemu,mac=$gho->{Ether}' ]
> +vif = [ 'type=ioemu,model=e1000,mac=$gho->{Ether}' ]
This should also be in its own patch, with the reasoning in commit log.
> #
> on_poweroff = 'destroy'
> on_reboot = '$onreboot'
> @@ -2036,4 +2047,14 @@ sub guest_editconfig_nocd ($$) {
> });
> }
>
> +sub guest_editconfig_cd ($) {
> + my ($gho) = @_;
> + guest_editconfig($gho->{Host}, $gho, sub {
> + if (m/^\s*boot\s*= '\s*d\s*c\s*'/) {
> + s/dc/cd/;
> + }
> + s/^on_reboot.*/on_reboot='restart'/;
> + });
> +}
> +
> 1;
> diff --git a/sg-run-job b/sg-run-job
> index 2cf810a..0898113 100755
> --- a/sg-run-job
> +++ b/sg-run-job
> @@ -288,6 +288,12 @@ proc run-job/test-pair {} {
> # run-ts . remus-failover ts-remus-check src_host dst_host +
> debian
> }
>
> +proc need-hosts/test-nested {} {return host}
> +proc run-job/test-nested {} {
> + run-ts . = ts-nested-L1-debian-install
> + run-ts . = ts-guest-destroy + host nested
> +}
> +
> proc test-guest-migr {g} {
> if {[catch { run-ts . = ts-migrate-support-check + host $g }]} return
>
> diff --git a/ts-nested-L1-debian-install b/ts-nested-L1-debian-install
> new file mode 100755
> index 0000000..4f6c5fa
> --- /dev/null
> +++ b/ts-nested-L1-debian-install
Looks like you're pulling in different functions from various parts of
OSSTest into this script. It makes this script very hard to maintain.
Why don't you just use them directly?
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |