Re: [Xen-devel] [PATCH v4] Autoconf: add options to enable spice and usbredir in qemu upstream

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2014-04-11 at 16:40 +0200, Fabio Fantoni wrote:
> Added configure options to enable spice and usbredir to upstream
> qemu build done with xen.
> 
> Note: this is a quick update of 2 years old patch,

Please provide references when reposting such old patches.

>  I wait feedback
> and I'll improve it if needed before add missed tool/configure
> update and testing the patch.
> 
> Signed-off-by: Fabio Fantoni <fabio.fantoni@xxxxxxx>
> 
> ---
> 
> Changes in v4:
> - removed upstream qemu debug option already included by default if
>   tools debug is enabled.
> - refresh for latest xen unstable git
> ---
>  README             |    6 ++++++
>  config/Tools.mk.in |    1 +
>  tools/Makefile     |    1 +
>  tools/configure.ac |    8 ++++++++
>  4 files changed, 16 insertions(+)
> 
> diff --git a/README b/README
> index 9bbe734..9a3fc34 100644
> --- a/README
> +++ b/README
> @@ -72,6 +72,12 @@ disabled at compile time:
>      * cmake (if building vtpm stub domains)
>      * markdown
>      * figlet (for generating the traditional Xen start of day banner)
> +    * Dev of spice protocol (e.g. libspice-protocol-dev)
> +      Required to build Spice for qemu upstream if enabled with configure
> +    * Dev of spice server (e.g. libspice-server-dev)
> +      Required to build Spice for qemu upstream if enabled with configure
> +    * Dev of usb redirection (e.g. libusbredir-dev). Required to build usb
> +      redirection for qemu upstream if enabled with configure

Everywhere else in this file says "Development install of FOO", which
reads more naturally.

>  Second, you need to acquire a suitable kernel for use in domain 0. If
>  possible you should use a kernel provided by your OS distributor. If
> diff --git a/config/Tools.mk.in b/config/Tools.mk.in
> index 0bdf37a..f25d065 100644
> --- a/config/Tools.mk.in
> +++ b/config/Tools.mk.in
> @@ -54,6 +54,7 @@ CONFIG_SEABIOS      := @seabios@
>  CONFIG_QEMU_TRAD    := @qemu_traditional@
>  CONFIG_QEMU_XEN     := @qemu_xen@
>  CONFIG_BLKTAP1      := @blktap1@
> +CONFIG_QEMUU_ADD_PAR:= @qemuu_add_par@
>  
>  #System options
>  ZLIB                := @zlib@
> diff --git a/tools/Makefile b/tools/Makefile
> index c3cfb38..254a410 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -203,6 +203,7 @@ subdir-all-qemu-xen-dir: qemu-xen-dir-find
>               --disable-docs \
>               --disable-guest-agent \
>               --python=$(PYTHON) \
> +        $(CONFIG_QEMUU_ADD_PAR) \

Whitespace.

A more meaningful name would be something like
"QEMUU_EXTRA_CONFIGURE_ARGS".

>               $(IOEMU_CONFIGURE_CROSS); \
>       $(MAKE) all
>  
> diff --git a/tools/configure.ac b/tools/configure.ac
> index 00fb47b..e65cb45 100644
> --- a/tools/configure.ac
> +++ b/tools/configure.ac
> @@ -59,6 +59,14 @@ AX_ARG_DEFAULT_ENABLE([seabios], [Disable SeaBIOS])
>  AX_ARG_DEFAULT_ENABLE([debug], [Disable debug build of tools])
>  AX_ARG_DEFAULT_DISABLE([blktap1], [Enable blktap1 tools])
>  
> +AC_ARG_ENABLE([qemuu-spice],
> +[ --enable-qemuu-spice    Enable Spice build on qemu upstream],
> +[qemuu_add_par+=" --enable-spice"])
> +AC_ARG_ENABLE([qemuu-usbredir],
> +[ --enable-qemuu-usbredir Enable usb redirection build on qemu upstream],
> +[qemuu_add_par+=" --enable-usb-redir"])
> +AC_SUBST(qemuu_add_par)

Lack of whitepace here makes it unreadable.

And foo+="" appears to be a bashism:
        $ /bin/dash 
        $ foo=a
        $ foo+=b
        /bin/dash: 2: foo+=b: not found

I think this would break configure on a default Debian install which
uses dash as /bin/sh.

> +
>  AC_ARG_ENABLE([qemu-traditional],
>      AS_HELP_STRING([--enable-qemu-traditional],
>                     [Enable qemu traditional device model, (DEFAULT is on for 
> x86, otherwise off)]),,[



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