Re: [PATCH XTF] tests: add fep test

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 17/06/16 15:21, Wei Liu wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

LGTM, although a couple of comments.

> ---
>  tests/fep/Makefile | 12 ++++++++++++
>  tests/fep/main.c   | 31 +++++++++++++++++++++++++++++++

Please add this to the test index in docs/all-tests.dox

>  2 files changed, 43 insertions(+)
>  create mode 100644 tests/fep/Makefile
>  create mode 100644 tests/fep/main.c
>
> diff --git a/tests/fep/Makefile b/tests/fep/Makefile
> new file mode 100644
> index 0000000..8702123
> --- /dev/null
> +++ b/tests/fep/Makefile
> @@ -0,0 +1,12 @@
> +MAKEFLAGS += -r
> +ROOT := $(abspath $(CURDIR)/../..)
> +
> +include $(ROOT)/build/common.mk
> +
> +NAME      := fep
> +CATEGORY  := utility
> +TEST-ENVS := $(HVM_ENVIRONMENTS)

This really doesn't need to be all HVM environments.  FEP is a property
of the HVM container, not of the running mode of the domain.  hvm32
would be fine here, and the most simple option.

> +
> +obj-perenv += main.o
> +
> +include $(ROOT)/build/gen.mk
> diff --git a/tests/fep/main.c b/tests/fep/main.c
> new file mode 100644
> index 0000000..34a93c0
> --- /dev/null
> +++ b/tests/fep/main.c
> @@ -0,0 +1,31 @@
> +/**
> + * @file tests/fep/main.c
> + * @ref test-fep
> + *
> + * @page test-fep FEP
> + *
> + * Returns SUCCESS if FEP is available, FAILURE if not.

This is the content one will find from the test index, and as such,
should be the most complete.  At the very least, I would add a sentence
explaining what FEP is.

~Andrew

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