[RFC] osstest: add a cd-{insert/eject} test for Debian HVM

Commit Message

Roger Pau Monné Feb. 18, 2016, 6:10 p.m. UTC

It could/should be expanded to other guest types, but at least it's a start.
It should also test that the guest can correctly access the disk contents.

The only supported toolstack for this test is xl.

Signed-off-by: Roger Pau Monné <[e-mail]>
---
CC: Ian Jackson <[e-mail]>
---
 Osstest/TestSupport.pm  | 17 +++++++++++++++++
 Osstest/Toolstack/xl.pm | 18 ++++++++++++++++++
 sg-run-job              |  1 +
 ts-hvm-cd               | 31 +++++++++++++++++++++++++++++++
 4 files changed, 67 insertions(+)
 create mode 100644 ts-hvm-cd

Comments

Ian Jackson Feb. 18, 2016, 7:21 p.m. UTC | #1

Roger Pau Monne writes ("[PATCH RFC] osstest: add a cd-{insert/eject} test for Debian HVM"):
> It could/should be expanded to other guest types, but at least it's a start.
> It should also test that the guest can correctly access the disk contents.
> 
> The only supported toolstack for this test is xl.

Thanks.

> +# Both the insert/eject functions hardcode 'hdc' as the virtual cdrom unit,
> +# if more_prepareguest_hvm is changed to use a different unit this is doomed
> +# to fail.

I think this hardcoding should be done closer to the source.  Ie, in
ts-hvm-cd.  The vdev to be ejected and inserted should be passed as
parameter to the toolstack functions.

The same goes for the iso image, I think.

BUT:

Regarding the vdev, I think it would be much better to get the vdev
information from a runvar.  ts-debianhvm-install could store the cd
vdev and your ts- script could read it.

Regarding the iso image: I think it's not a good idea to use the
install iso image.  Because, we would like to be able to carry on with
the rest of the test job if this step fails.  And we may not know
whether the cd has been inserted.  If it has, and it's the install cd,
things might boot off it and redo the install.

Using the empty iso image would be better.  (See guest_editconfig_nocd
in TestSupport.pm.)


> +sub guest_eject_cd ($) {
> +    my ($gho) = @_;
> +    my $ho = $gho->{Host};
> +    die "xl is the only supported toolstack" if $tsname neq 'xl';
> +    toolstack($ho)->cd_eject($gho);

This is not appropriate.  You could do this by providing dummy
methods in the toolstack classes.  But TBH if you just leave out the
check, what happens is that the call will go to an unimplemented
method and bomb out anyway.

> diff --git a/sg-run-job b/sg-run-job
> index 3e0f966..43a141f 100755
> --- a/sg-run-job
> +++ b/sg-run-job
> @@ -320,6 +320,7 @@ proc run-job/test-rhelhvm {} {
>  proc need-hosts/test-debianhvm {} { return host }
>  proc run-job/test-debianhvm {} {
>      run-ts . = ts-debian-hvm-install
> +    run-ts . = ts-hvm-cd

I think that it would be worth making some effort to avoid blocking
the whole test job if this fails.  (Since AIUI currently we expect
this to fail on some branches.)

>      test-guest debianhvm

And then ideally this new call would be in test-guest.

Also I am not happy with the script name.  ts-cd-eject ?  (The script
name turns into a test step name (by default) and the test step name
ought not to vary according to whether the job is HVM or not.)


> +guest_eject_cd($gho)
> +sleep(10)
> +guest_insert_cd($gho)
> +sleep(10)
> +guest_eject_cd($gho)

How hard would it be to provide a dummy image and then check, in the
guest, that the guest can read it ?


Ian.

Patch

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 2141905..4445a04 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -2508,6 +2508,23 @@  sub guest_editconfig_nocd ($$) {
     });
 }
 
+# Both the insert/eject functions hardcode 'hdc' as the virtual cdrom unit,
+# if more_prepareguest_hvm is changed to use a different unit this is doomed
+# to fail.
+sub guest_eject_cd ($) {
+    my ($gho) = @_;
+    my $ho = $gho->{Host};
+    die "xl is the only supported toolstack" if $tsname neq 'xl';
+    toolstack($ho)->cd_eject($gho);
+}
+
+sub guest_insert_cd ($) {
+    my ($gho) = @_;
+    my $ho = $gho->{Host};
+    die "xl is the only supported toolstack" if $tsname neq 'xl';
+    toolstack($ho)->cd_insert($gho);
+}
+
 sub host_install_postboot_complete ($) {
     my ($ho) = @_;
     target_core_dump_setup($ho);
diff --git a/Osstest/Toolstack/xl.pm b/Osstest/Toolstack/xl.pm
index d956bcd..b25b19c 100644
--- a/Osstest/Toolstack/xl.pm
+++ b/Osstest/Toolstack/xl.pm
@@ -118,4 +118,22 @@  sub block_attach ($$$$) {
     target_cmd_root($ho, $cmd, 100);
 }
 
+sub cd_eject($$) {
+    my ($self,$gho) = @_;
+    my $ho = $self->{Host};
+    my $gn = $gho->{Name};
+    my $cmd = $self->{_VerboseCommand}." cd-eject $gn hdc";
+    target_cmd_root($ho, $cmd, 100);
+}
+
+sub cd_insert($$) {
+    my ($self,$gho) = @_;
+    my $ho = $self->{Host};
+    my $gn = $gho->{Name};
+    my $iso = $gho->{Rimage};
+    my $cmd = $self->{_VerboseCommand}." cd-insert $gn hdc $iso";
+    target_cmd_root($ho, $cmd, 100);
+}
+
+
 1;
diff --git a/sg-run-job b/sg-run-job
index 3e0f966..43a141f 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -320,6 +320,7 @@  proc run-job/test-rhelhvm {} {
 proc need-hosts/test-debianhvm {} { return host }
 proc run-job/test-debianhvm {} {
     run-ts . = ts-debian-hvm-install
+    run-ts . = ts-hvm-cd
     test-guest debianhvm
 }
 
diff --git a/ts-hvm-cd b/ts-hvm-cd
new file mode 100644
index 0000000..5776afd
--- /dev/null
+++ b/ts-hvm-cd
@@ -0,0 +1,31 @@ 
+#!/usr/bin/perl -w
+# This is part of "osstest", an automated testing framework for Xen.
+# Copyright (C) 2009-2016 Citrix 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
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+# 
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+# 
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+use strict qw(vars);
+use DBI;
+use Osstest;
+use Osstest::TestSupport;
+
+tsreadconfig();
+
+our $gho;
+
+guest_eject_cd($gho)
+sleep(10)
+guest_insert_cd($gho)
+sleep(10)
+guest_eject_cd($gho)