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

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

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.

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